-
Notifications
You must be signed in to change notification settings - Fork 125
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Pybind11 Imath Frustum #459
Fix Pybind11 Imath Frustum #459
Conversation
Signed-off-by: Todica Ionut <[email protected]>
Signed-off-by: Todica Ionut <[email protected]>
Signed-off-by: Todica Ionut <[email protected]>
Signed-off-by: Todica Ionut <[email protected]>
Signed-off-by: Todica Ionut <[email protected]>
Signed-off-by: Todica Ionut <[email protected]>
Signed-off-by: Todica Ionut <[email protected]>
Signed-off-by: Todica Ionut <[email protected]>
Signed-off-by: Todica Ionut <[email protected]>
Signed-off-by: Todica Ionut <[email protected]>
Signed-off-by: Todica Ionut <[email protected]>
Signed-off-by: Todica Ionut <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the speedy response, and yes, the previous PR replaced the Imath::
with ``IMATH_NAMESPACE::` to support the case where Imath is built with a custom namespace.
Looks like the template is still not quite right here.
void register_frustum(pybind11::module& m, const char *name) | ||
{ | ||
pybind11::class_<T> c(m, name); | ||
c.def(pybind11::init<>(), "Uninitialized by default") | ||
.def(pybind11::init<T>(), pybind11::arg("frustum"), "Copy constructor") | ||
.def(pybind11::init<S>(), pybind11::arg("nearPlane"), pybind11::arg("farPlane"), pybind11::arg("fovx"), pybind11::arg("aspect"), "Initialize with basic frustum properties") | ||
.def(pybind11::init<const S, S, S, S>(), pybind11::arg("nearPlane"), pybind11::arg("farPlane"), pybind11::arg("fovx"), pybind11::arg("aspect"), "Initialize with basic frustum properties") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nearPlane, farPlane, fovx, and aspect are float/double, but S in this context is M44f/M44d, right?
Signed-off-by: Todica Ionut <[email protected]>
Looks like you added a 4th argument to the template instantiation but not the declaration, the build is still failing. Is this somehow compiling on your end? |
Signed-off-by: Todica Ionut <[email protected]>
Signed-off-by: Todica Ionut <[email protected]>
Signed-off-by: Todica Ionut <[email protected]>
Signed-off-by: Todica Ionut <[email protected]>
Signed-off-by: Todica Ionut <[email protected]>
Signed-off-by: Todica Ionut <[email protected]>
Ci temporarily:
@cary-ilm I merged. CI checks failed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This gets the build working again, I'm going to merge as is. I'll be away from the project Dec 19-Jan 12, but can help resolve the issues after that.
not working